-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[ES|QL] Add MATCH_PHRASE #127661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[ES|QL] Add MATCH_PHRASE #127661
Conversation
dabef06
to
13ccda7
Compare
Hi @kderusso, I've created a changelog YAML for you. |
8f01955
to
d02864e
Compare
|
||
MatchPhrase can use <<esql-function-named-params,function named parameters>> to specify additional options for the | ||
match_phrase query. | ||
All <<query-dsl-match-query-phrase,match_phrase>> query parameters are supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: When I try to link to the correct doclink here, I get:
Invalid link key <<match-phrase-field-params,match_phrase query parameters>>
|
||
**Description** | ||
|
||
Use `MATCH_PHRASE` to perform a [match_phrase query](/reference/query-languages/query-dsl/query-dsl-match-query.md#query-dsl-match-query-phrase) on the specified field. Using `MATCH_PHRASE` is equivalent to using the `match_phrase` query in the Elasticsearch Query DSL. MatchPhrase can be used on [text](/reference/elasticsearch/mapping-reference/text.md) fields, as well as other field types like keyword, boolean, or date types. MatchPhrase is not supported for [semantic_text](/reference/elasticsearch/mapping-reference/semantic-text.md) or numeric types. MatchPhrase can use [function named parameters](/reference/query-languages/esql/esql-syntax.md#esql-function-named-params) to specify additional options for the match_phrase query. All [match_phrase](/reference/query-languages/query-dsl/query-dsl-match-query.md#query-dsl-match-query-phrase) query parameters are supported. `MATCH_PHRASE` returns true if the provided query matches the row. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why we're picking up match
query docs in the generated documentation here, we should be picking up query-dsl-match-query-phrase.md
.
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Pinging @elastic/search-eng (Team:SearchOrg) |
Pinging @elastic/search-relevance (Team:Search - Relevance) |
@@ -1,4 +1,7 @@ | |||
* [preview] [`KQL`](../../functions-operators/search-functions.md#esql-kql) | |||
* [preview] [`MATCH`](../../functions-operators/search-functions.md#esql-match) | |||
* [preview] [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to modify this file, because the function is just available in snapshot.
At most you can comment this one with a %
as we do for the term function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out, thanks
:::{include} ../_snippets/functions/layout/kql.md | ||
::: | ||
|
||
:::{include} ../_snippets/functions/layout/match.md | ||
::: | ||
|
||
:::{include} ../_snippets/functions/layout/match_phrase.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again I don't think we need this since the function is not available outside snapshots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are missing validation tests in VerifierTests
for cases like:
- what happens when
match_phrase
is used afterSTATS
- this should fail with a validation error - what happens when we pass invalid options to
match_phrase
- same, we have a validation error - what happens when
match_phrase
receives a column as argument that is not a mapped field - again a validation error
there are other cases that we might need to test - please take a look at what we did for match
in VerifierTests and add similar tests.
x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml
Outdated
Show resolved
Hide resolved
return new MatchPhraseQuery(source(), fieldName, queryAsObject(), matchPhraseQueryOptions()); | ||
} | ||
|
||
public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate of:
Lines 468 to 475 in f4b6086
public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) { | |
String fieldName = fieldAttribute.name(); | |
if (fieldAttribute.field() instanceof MultiTypeEsField multiTypeEsField) { | |
// If we have multiple field types, we allow the query to be done, but getting the underlying field name | |
fieldName = multiTypeEsField.getName(); | |
} | |
return fieldName; | |
} |
Either use the Match
one or move them both to another place, like FullTextFunction
.
return fieldName; | ||
} | ||
|
||
public static FieldAttribute fieldAsFieldAttribute(Expression field) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate of:
Lines 477 to 484 in f4b6086
public static FieldAttribute fieldAsFieldAttribute(Expression field) { | |
Expression fieldExpression = field; | |
// Field may be converted to other data type (field_name :: data_type), so we need to check the original field | |
if (fieldExpression instanceof AbstractConvertFunction convertFunction) { | |
fieldExpression = convertFunction.field(); | |
} | |
return fieldExpression instanceof FieldAttribute fieldAttribute ? fieldAttribute : null; | |
} |
Either use the Match
one or move them both to another place, like FullTextFunction
.
} | ||
|
||
try { | ||
matchPhraseQueryOptions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the resolveOptions
function could use a refactor - because most of it is duplicated in Match
, MultiMatch
, QueryString
and here. But I am okay with doing that as a follow up.
failures.add( | ||
Failure.fail( | ||
mp.field(), | ||
"[{}] {} cannot operate on [{}], which is not a field from an index mapping", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's get some tests for this particular case in VerifierTests
Adds
MATCH_PHRASE
support to ES|QL.Some examples of how to use
MATCH_PHRASE
: